Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add ability to indicate bad args #302

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

add ability to indicate bad args #302

wants to merge 1 commit into from

Conversation

klnusbaum
Copy link
Contributor

@klnusbaum klnusbaum commented Apr 6, 2018

There are instances where we'd like to be able to indicate to upstream connectors that the particular args they received were invalid (i.e. the request was incorrect). Having this error would allow us, for instance, to catch Cassandra specific errors and bubble them up to our gateway handler, which could then in turn properly encapsulate the error as a BadRequestError instead of an internal service error.

I have some code serverside that I want to add to the Cassandra connector. This code has cassandra specific input validation and I'd like to be able to indicate to the handler that this error the cassandra connector is returning isn't the result of an internal server error, but rather that it was given some bad inputs.

Copy link
Contributor

@mattgode mattgode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would also suggest we also export error codes that can be referenced by the gateway. Right now we are using http error codes, but this is a somewhat implicit contract between the gateway and the client.

@klnusbaum
Copy link
Contributor Author

@phliar @junchaoWu what do you folks think about adding this ability? I'm actually kind of on the fence about it.

@phliar
Copy link
Contributor

phliar commented Apr 9, 2018

If we speak in HTTP codes, this would be replacing a 500 with a 400, which makes perfect sense.

@mattgode Do you mean documenting which endpoints can return which errors? (I assume we do not actually want to export HTTP codes as part of the DOSA API.)

@mattgode
Copy link
Contributor

@phliar the fact that they are http error codes is somewhat immaterial. I was suggesting the use of constants to indicate which codes should be sent over the wire to properly map to an error that is defined in DOSA .

Specifically here I am referring to our use of http error codes in the YARPC connector. For example, there is an implicit contract where we know a 404 status code maps to a not found error but I think this connector export some error codes (they can still be http, it doesn't really matter) such that we can have a more explicit contract between gateway and YARPC client.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants